Skip to content

chore: refine Copilot PR interface review prompt [no-ci]#157

Merged
lukeocodes merged 1 commit intomainfrom
chore/refine-copilot-pr-review-prompt
May 6, 2026
Merged

chore: refine Copilot PR interface review prompt [no-ci]#157
lukeocodes merged 1 commit intomainfrom
chore/refine-copilot-pr-review-prompt

Conversation

@lukeocodes
Copy link
Copy Markdown
Member

Refines .github/prompts/pr-interface-review.md based on Copilot review feedback raised during the original rollout:

  1. Step 1 / Step 5 consistency — Step 1 said "ignore tests except deleted" but Step 5 also requires reviewing added tests that prove compat shims. Broadened Step 1 to cover added, deleted, and modified-to-remove tests. (raised on deepgram/deepgram-go-sdk#329)

  2. Tier 6 broadened — original wording only mentioned same-named local symbols as collision vectors. Now also covers glob/namespace imports (Python from X import *, Rust use X::*, Go dot-imports, TypeScript import * as destructuring). (raised on deepgram/deepgram-js-sdk#495)

  3. Fence language — the template fenced block now declares markdown to satisfy markdownlint MD040. (raised on deepgram/deepgram-go-sdk#329)

Canonical source: deepgram/dx-stack. Rollout context: docs/copilot-prompts.md.

[no-ci] — docs-only refinement to a previously-merged file.

Copilot AI review requested due to automatic review settings May 6, 2026 13:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refines the Copilot “PR interface & contract review” prompt to better align its instructions around test-handling, clarify Tier-6 collision vectors, and satisfy markdownlint fencing requirements.

Changes:

  • Updates Step 1 to explicitly call out tests that were added, deleted, or modified to remove a case (to align with Step 5’s test-audit requirements).
  • Expands Tier 6 wording to include additional name-collision vectors (glob/namespace/dot-import patterns across languages).
  • Sets the template code fence language to markdown to satisfy markdownlint MD040.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

| 🔍 **4** | **Type tightening / silent risk** | A type was narrowed (e.g. `str` → enum), a default removed, a docstring contract changed, or behavior subtly shifted. Compiles fine; may surprise users. |
| ➕ **5** | **Purely additive** | New optional field, new optional parameter with default, new method, new exported type. No existing caller can break. |
| 🆕 **6** | **Brand-new public type** | Entirely new type/class/symbol with no predecessor. Only breaks callers who happened to define a same-named local symbol. |
| 🆕 **6** | **Brand-new public type** | Entirely new type/class/symbol with no predecessor. Only breaks callers via name collision — same-named local symbol, glob imports (`from X import *`, `use X::*`), Go dot-imports, or TypeScript `import * as` destructuring. |
@lukeocodes lukeocodes merged commit 771a5da into main May 6, 2026
20 of 22 checks passed
@lukeocodes lukeocodes deleted the chore/refine-copilot-pr-review-prompt branch May 6, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants